Repository navigation
fix(websocket): reconnect after clean server close (INT-1697) - #751
ed-lepedus-thenvoi wants to merge 1 commit into
Conversation
AlexanderZ-Band
left a comment
There was a problem hiding this comment.
Observations
ReconnectPolicy(reconnect_on_normal_close=True) makes both 1000 and 1001 reconnect. band-sdk-core 2.8.0 classify_close marks 1000 terminal and 1001 retryable. A non-retryable supersede is what stops the session; the 1001 after it is ignored because the session is already Dead. classify_upgrade marks 403 terminal.
After the first successful connect, _finalize_successful_connect sets auto_reconnect = True. Later closes are classified by phoenix-channels-python-client 0.2.5, where reconnect_on_normal_close defaults to False, so both 1000 and 1001 stop the client. on_disconnect receives None for those closes. The initial-connect path calls Session.on_socket_close with close_code=None.
The open phoenix-channels stack (#59–#66) keeps that decision inside _decide_reconnect. #62 leaves reconnect_on_normal_close False. #63 keeps retrying refused handshakes inside the supervisor, so a post-connect 403 never reaches on_upgrade_rejected. #64 leaves socket reconnect policy unchanged.
Suggestion
Close this PR. Track the 1001 case on INT-1355: keep Session in charge after the first connect, pass the real close code to on_socket_close, and pass a rejected upgrade to on_upgrade_rejected. A server 1000 staying down is the core rule; that production case belongs on PLT-1573.
Why
A server-initiated clean WebSocket close (1000 or 1001) can stop a long-running Python agent rather than reconnect it. This was reproduced with band-sdk 4.0.0 through a fault proxy, and a production close 1000 previously left an agent unable to receive messages for roughly 34 hours. An abnormal close or service-restart close (1012) does reconnect.
Proposed behavior
agent_control:supersedepush.disconnect()remains terminal.Policy decision for SDK owners: Is server-initiated 1001 reconnectable absent a terminal push, as the platform WebSocket contract specifies? This draft proposes yes. A lost supersede push can cause retries, so cooldown and
Retry-Afterhandling need explicit review. This reverses the test-only, unmerged #441 proposal that pinned both 1000 and 1001 terminal.Scope and limits
This is a focused SDK reconnect-policy change, not a replacement for Session ownership of the Phoenix client's post-connect retry loop (INT-1355). The gateway's independent dead-link watchdog remains a backstop. A September lab stop produced 1000, whereas an October three-node rolling stop produced 1012; production deploy close codes have not been independently confirmed. Do not interpret this PR as evidence that every deployment triggers the bug.
Tracking: INT-1697; gateway report #43; platform follow-up PLT-1573.
Verification